Skip to content

fix: fp-check Stop hooks return JSON instead of plain text#137

Open
bluzername wants to merge 1 commit intotrailofbits:mainfrom
bluzername:fix/fp-check-stop-hook-json
Open

fix: fp-check Stop hooks return JSON instead of plain text#137
bluzername wants to merge 1 commit intotrailofbits:mainfrom
bluzername:fix/fp-check-stop-hook-json

Conversation

@bluzername
Copy link
Copy Markdown

Problem

The fp-check plugin's Stop and SubagentStop hooks cause "JSON validation failed" error every time Claude Code exits a session. The hooks use type: "prompt" which expects the LLM to return structured JSON, but the prompt instructions tell it to return plain text like 'approve' or 'block'.

I was getting this error on every session exit after installing fp-check and it was confusing because the plugin wasnt even being used in those sessions.

Root Cause

In plugins/fp-check/hooks/hooks.json, both hooks have prompts ending with:

return 'block' with the specific gaps.
return 'approve'.

But Claude Code expects prompt-type hooks to return valid JSON like:

{"decision": "approve"}

Fix

Updated the return instructions in both hooks to ask for JSON format:

Stop hook (line 10):

return JSON: {"decision": "block", "reason": "<specific gaps>"}
return JSON: {"decision": "approve"}

SubagentStop hook (line 22):
Same pattern.

Also added IMPORTANT: Return valid JSON only, no text outside the JSON object. to both prompts to make sure the LLM doesnt add extra text around the JSON.

Impact

This error happens on EVERY session exit when fp-check is installed, even for sessions that have nothing to do with false positive verification. The fix makes both hooks return proper JSON so Claude Code can parse the response correctly.

Closes #131

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@dguido
Copy link
Copy Markdown
Member

dguido commented Mar 31, 2026

Automated Review — PR #137

Flagged for human review (touches hooks)

Review Findings

The fix is correct and addresses a real bug. Issue #131 documents that prompt-type Stop/SubagentStop hooks must return JSON with a decision field, but the existing prompts instructed the LLM to return plain text ('approve' / 'block'), causing "JSON validation failed" errors on every session exit.

Changes reviewed:

  1. Stop hook prompt (line 10): Return instructions changed from plain text (return 'block' / return 'approve') to JSON format (return JSON: {"decision": "block", "reason": "..."} / return JSON: {"decision": "approve"}). Added IMPORTANT: Return valid JSON only, no text outside the JSON object. reinforcement.

  2. SubagentStop hook prompt (line 22): Same fix applied consistently.

  3. Description field (line 2): No-op change — the em-dash character () is identical in both versions, just represented differently in the diff (\u2014 vs literal). Harmless.

Correctness assessment:

  • The JSON format {"decision": "approve"} and {"decision": "block", "reason": "..."} matches the Claude Code hooks specification for Stop/SubagentStop event responses.
  • The IMPORTANT: Return valid JSON only suffix is a good safeguard against LLM wrapping the JSON in markdown or explanatory text.
  • Both hooks are updated consistently.

Potential concern — version not bumped:

  • plugin.json and marketplace.json both show version 1.0.0. Per the PR checklist, substantive changes should increment the version so clients pick up the update. This is a bug fix that affects every session, so a version bump to 1.0.1 in both files seems warranted. Recommend the author add this before merge.

No security concerns. The hooks only inspect conversation content and return approve/block decisions — no file system access, no command execution, no data exfiltration vectors.

Validation

  • validate_codex_skills.py: PASS (61 plugin skills validated against 62 Codex entries)
  • validate_plugin_metadata.py: PASS (all 36 plugins in sync)
  • hooks.json JSON syntax: PASS (valid JSON)

Changes Made

None — review-only for complex PRs.


Reviewed by Claude Code

@dguido
Copy link
Copy Markdown
Member

dguido commented Mar 31, 2026

@bluzername please sign the CLA so we can consider this contribution! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[fp-check] Stop hook returns non-JSON output causing JSON validation error in Claude Code

4 participants